Skip to content

feat(datasets): add update subcommand to rename label or table_name#65

Merged
zfarrell merged 6 commits into
mainfrom
feat/datasets-update-cli
Apr 29, 2026
Merged

feat(datasets): add update subcommand to rename label or table_name#65
zfarrell merged 6 commits into
mainfrom
feat/datasets-update-cli

Conversation

@zfarrell
Copy link
Copy Markdown
Contributor

@zfarrell zfarrell commented Apr 28, 2026

Adds hotdata datasets update <ID> [--label X] [--table-name Y] so users can rename a dataset without recreating it. Closes the gap motivated by hotdata-dev/runtimedb#371 (auto-generated table names that diverged from labels).

Notes for reviewers

  • The src/api.rs hunks beyond the new put() method are unrelated rustfmt cleanup that got picked up automatically when the file was touched. The only behavior change in that file is the added put() helper.
  • --pinned-version / --unpin flags were intentionally scoped out — tracked in Add --pinned-version / --unpin to datasets update #66.

@sentry
Copy link
Copy Markdown

sentry Bot commented Apr 28, 2026

Codecov Report

❌ Patch coverage is 48.17518% with 71 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/datasets.rs 50.00% 45 Missing ⚠️
src/api.rs 56.75% 16 Missing ⚠️
src/main.rs 0.00% 10 Missing ⚠️

📢 Thoughts on this report? Let us know!

Comment thread src/datasets.rs Outdated
Comment thread src/datasets.rs
claude[bot]
claude Bot previously approved these changes Apr 28, 2026
claude[bot]
claude Bot previously approved these changes Apr 28, 2026
Comment thread src/datasets.rs
claude[bot]
claude Bot previously approved these changes Apr 28, 2026
claude[bot]
claude Bot previously approved these changes Apr 29, 2026
Comment thread src/datasets.rs
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Blocking Issues

  • src/datasets.rs:527: println! for "Dataset updated" corrupts stdout in -o json / -o yaml modes — any pipe like hotdata datasets update ... -o json | jq will fail because the human-readable line is not valid JSON.

Action Required

Change line 527 from println! to eprintln! so the status message goes to stderr and stdout stays clean for structured output. This matches what you described in your reply to thread #3157593891 — the final commit appears to have regressed it back to println!.

Comment thread src/datasets.rs Outdated
let d: UpdateResponse = api.put(&format!("/datasets/{dataset_id}"), &body);

use crossterm::style::Stylize;
println!("{}", "Dataset updated".green());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocking: println! here emits "Dataset updated" to stdout before the JSON/YAML payload, which breaks piping in structured output modes. Running hotdata datasets update ... -o json | jq will fail because the first line is not valid JSON.

Change to eprintln! to keep stdout clean — this is what you described as intentional in thread #3157593891, but the final commit (fb8af95) regressed it back to println!.

Suggested change
println!("{}", "Dataset updated".green());
eprintln!("{}", "Dataset updated".green());

Reverts fb8af95. The suggestion changed eprintln! to println!, which
puts the human prelude on stdout and breaks `-o json` / `-o yaml`
piping (e.g. `... -o json | jq` fails with a parse error because the
status line precedes the JSON).
@zfarrell zfarrell merged commit ae8ec55 into main Apr 29, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant